Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Bug 809974: Display "Last Updated" date and time in Device Info. r=kaze #6560

Merged
merged 5 commits into from Dec 7, 2012

Conversation

marshall
Copy link
Contributor

@marshall
Copy link
Contributor Author

r? @fabi1cazenave

*/
#last-update-date {
font-size: 1.4rem;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s my only nit: why do we need a specific rule for this item? I have to ask, as we’re trying to improve the UI consistency in the Settings app…

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the date and time will overlap the "Last updated" label otherwise. I hunted around for a classname I could reuse but didn't see any -- feel free to enlighten me :). I also found a bug if no Last Updated date was found, so I will push that fix to my branch soon.

Do we have any automated way to test the settings UI code?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, I’d suggest to put the date below the label instead of next to it — think of other locales for which the text will be even trickier to fit.

<li>
  <small>2012-11-24</small>
  <a> Last updated </a>
</li>

@marshall
Copy link
Contributor Author

@jcarpenter Do you have a preference for the text of Device Info > Last Updated when there wasn't a last updated date / time? Some possibilities:

"No update applied yet"
"Never"
.. or just leave it empty ?

@jcarpenter
Copy link
Contributor

@marshall

Do you have a preference for the text of Device Info > Last Updated when there wasn't a last updated date / time?

What about the device install date? Or empty is also fine. Easier for l10n team.

@jcarpenter
Copy link
Contributor

@fabi1cazenave

In this case, I’d suggest to put the date below the label instead of next to it — think of other locales for which the text will be even trickier to fit.

Let's keep it on the right. Making an exception for this one field would break the consistency of the left-right layout. In the relatively unlikely event that the date exceeds the available space, we can wrap the text.

@fabi1cazenave
Copy link
Contributor

@jcarpenter it is very likely that the targeted locales will require more space than English to display the translation of “last update”, and there are two problems with this approach:

  • for these items where the heading and the data are on the same line, we currently have no way to wrap the heading automatically (that’s why we only use this when we’re sure it won’t overlap);
  • wrapping the heading text would not help for Spanish or Portuguese: for these latin languages the important word (“update”) will be the last one, thus dropped if the text is wrapped.

Of course, there are other panels where the data is below the heading (e.g. main panel). And for the “Software” item above this “Last Updated” one, @lco has already suggested to use two lines instead of one: https://bugzilla.mozilla.org/show_bug.cgi?id=808892

However, if the priority is to make this “Device information” panel look good in English then we need a better way to divide the space between the heading and the data — e.g. use a fixed width of 50% for heading and data, and wrap the text accordingly. As you can guess, I’d still find this silly.

@marshall
Copy link
Contributor Author

@jcarpenter @fabi1cazenave Here's a screenshot of what it currently looks like, if it helps
last updated

@fabi1cazenave
Copy link
Contributor

@marshall the date string size is stable and predictable, but the heading string size is very dependent on the locale. Your screenshot looks nice for English but I’d much, much prefer to put this on two lines to ensure it’ll still be readable for all locales.

@marshall
Copy link
Contributor Author

@fabi1cazenave @jcarpenter new screenshot
last updated

@fabi1cazenave
Copy link
Contributor

this is fine to me

@fabi1cazenave
Copy link
Contributor

@marshall would you please attach this PR to bug 809974 so I can r+ it and merge it?

@jcarpenter
Copy link
Contributor

for these items where the heading and the data are on the same line, we currently have no way to wrap the heading automatically (that’s why we only use this when we’re sure it won’t overlap);
wrapping the heading text would not help for Spanish or Portuguese: for these latin languages the important word (“update”) will be the last one, thus dropped if the text is wrapped.
Of course, there are other panels where the data is below the heading (e.g. main panel). And for the “Software” item above this “Last Updated” one, @lco has already suggested to use two lines instead of one: https://bugzilla.mozilla.org/show_bug.cgi?id=808892

Suggested, but it has not been agreed upon internally by UX, and it was not planned for v1.

Long term we need a better solution to overruns and collisions. Other mobile OS avoid this by:

  • Being smart about localization
  • Automatically tightening character spacing
  • Automatically adding elipsis
  • Wrapping text lines

What we cannot do is hard code in exceptions case by case. We're going to miss some, inevitably. And it will look terrible if some fields are right aligned, and some are spread across two lines. Consistency matters.

I can live with this exception for v1, but in future please wait for UX approval before requesting merge, especially when my last comment explicitly asked for the opposite approach.

@fabi1cazenave
Copy link
Contributor

then let's go with the smaller font-size then.

@fabi1cazenave
Copy link
Contributor

@marshall please still open a bug and ask r? from somebody else. Thanks!

@vingtetun
Copy link
Contributor

@jcarpenter

  • Being smart about localization
    I don't have the feeling that the default design is making English strings that will translate shortly in other languages. Localizers spend a lot of time trying to keep the initial meaning of the string and to fit in the UI. Sadly this is hard for some languages that use strings longer than english, you should try b2g in french and try to navigate a bit in the settings...

So the first smart localization is on the original strings (aka original design), they need to be thinked so the translation will not be longer.

  • Tightening character spacing
    That would be nice to have but CSS does not have a magic property that does that if the fields does not fit in the container. You may want to add a proposal for letter-spacing: automatic. Doing that on a per-app basis is a fail too, it needs to be a standard fix.
  • Automatically adding ellipsis
    This is not a proof of being smart. It makes me upset when a screen contains ellipsis for default values. Is the phone not supposed to be designed to be usable in my language?
  • Wrapping text line
    This one sounds good too for me.

Anyway I don't consider that we should block on UX issue. It can be fixed into a different bug that is not blocking )or you can ask blocking on it if you want) but we need to resolve the functional issues first, even if the UI is not perfect yet.

@fabi1cazenave
Copy link
Contributor

Just realized that I understood “ellipsed” instead of “wrapped”, sorry for the confusion. I agree wrapping text would be far more acceptable that truncating it, but it would make the list item bigger (which you might not like).

Other mobile OS avoid this by […]

Other mobile OS have their own graphical widgets, with their pros and cons; at this stage of the project I’d much prefer to first implement what the platform can do easily.

@marshall
Copy link
Contributor Author

@fabi1cazenave I've attached the PR to Bug 809974 and marked you for r?

Just to be clear -- do I need to revert back to "on the same line" behavior in this pull request?

@evelynhung
Copy link
Contributor

@marshall
the patch can't be automatically merged probably because it conflicts with my latest commit of settings.js or index.html.
Could you please update to the latest Gaia and resolve the conflicts carefully? Thanks!

@fabi1cazenave
Copy link
Contributor

Just to be clear -- do I need to revert back to "on the same line" behavior in this pull request?

@marshall as my opinion regarding localization issues in the Settings app is not relevant, I’ll let you see that with UX. And since @evelynhung jumped in, I think it qualifies her for a review. :-)

Thanks for your work, sorry for the confusion here.

@evelynhung
Copy link
Contributor

Hi @marshall, in my PR #6771 I've made all fields in two lines, so I think your PR is fine, just need to resolve all conflicts so we can merge it.

@jcarpenter If the two-line display is not good for UX, please file another issue, we can address the problem there. Thanks!

@jcarpenter
Copy link
Contributor

@vingtetun

. . . we need to resolve the functional issues first, even if the UI is not perfect yet.

Agreed. I can live with the two lines approach for v1, if the code is already written and merged. We have bigger things to deal with.

Everything else in your response I agree with.

  • Smart localization is our best tool.
  • In the future, our English-speaking UX team should allow for extra space in our layouts, in anticipation of the longer strings of other languages.
  • In the future we can look into adding automatic adaptations, eg: elipsis, compressed letter spacing, and line wraps.

…pdated

Conflicts:
	apps/settings/index.html
	apps/settings/style/settings.css
@marshall
Copy link
Contributor Author

marshall commented Dec 4, 2012

@evelynhung I've merged and tested and all looks good

@marshall
Copy link
Contributor Author

marshall commented Dec 7, 2012

@evelynhung is this good to merge?

@fabi1cazenave
Copy link
Contributor

Merging, as @evelynhung has already r+’ed your patch.

fabi1cazenave added a commit that referenced this pull request Dec 7, 2012
Bug 809974: Display "Last Updated" date and time in Device Info. r=evelyn
@fabi1cazenave fabi1cazenave merged commit 46baef6 into mozilla-b2g:master Dec 7, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants